review pass: load-boundary hardening + codegen dedup#64
Merged
Conversation
Route IDB autosave restore through `project-file/classify-payload` + `sanitize-doc` so a corrupted or tampered autosave can't bypass the checks the file-open path uses. Strict-reject unknown `:version` so a future schema bump fails loud instead of partially installing the wrong shape. Cancel the watcher-armed 750ms timer on file load so a stale `save-now!` can't fire after `clear-autosave!`. Surface autosave write failures as `[:ui :autosave-failed?]` with a hidden ⚠ indicator in the toolbar. Extend `unsafe-findings` with an identifier-shape scanner so attr keys, binding prop names, binding `:field`, trigger `:action-ref`, payload `:field`, field-def `:name`, and action `:name` that carry characters unsafe for codegen are refused at the load boundary — closes the JS / CLJS string-injection vectors in both export plugins. Extract `action-ref-canonical-ns` + `action-ref-alias` to `export.model` (single source of truth shared by both plugins). Route `cljs_project/emit-sub-group-child` through `em/resolve-template-source` so the legacy fallback that re-derived `stateful-host-for-template` is gone. Replace the `volatile!` in `dnd/drag/marquee-hits` with an `into` + `keep` pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Code-review pass focused on load-boundary correctness, export-codegen safety, and shared-helper dedup. All ten findings from the review thread are addressed; no behavioural change for clean inputs.
Load boundary (storage)
idb/restore!returns the parsed payload only;pf/apply-autosave!runsclassify-payload+sanitize-docand refuses on spec / unsafe-findings failure. A corrupted or hand-tampered IDB entry can no longer bypass validation and silently install a malformed or hostile document.idb/deserializestrict-rejects unknown:version. Missing or future versions returnnilso a schema bump fails loud instead of partially installing the wrong shape. Newidb/supported-versionconstant.apply-loaded-project!cancels the watcher-armed autosave timer. The 750ms timer the swap synchronously arms is dropped so a stalesave-now!can't fire afterclear-autosave!completes.save-now!sets/clears[:ui :autosave-failed?]; toolbar renders a hidden⚠that unhides on failure (instead of a silentconsole.warn).main/inithas a.catch. An unexpected rejection in the init chain seeds an empty document and installs the watcher rather than leaving a blank page.Codegen safety (export)
sanitize.cljs.unsafe-findingsnow flags attr keys, binding prop names, binding:field, trigger:action-ref, payload:field, field-def:name, and action:namethat contain characters unsafe for codegen. Closes the JS / CLJS string-injection vectors a tampered project file could otherwise smuggle into emitted artifacts.Dedup (export)
em/action-ref-canonical-ns+em/action-ref-aliasextracted tobareforge.export.model. Bothcljs_projectandvanilla_js/codegennow delegate instead of carrying parallelsubs/name->ns-segmentdances.cljs_project/emit-sub-group-childnow routes throughem/resolve-template-source(the legacy fallback re-derivingstateful-host-for-templateis gone).Other
dnd/drag/marquee-hitsvolatile!→(into [] (keep …) (range n))per the CLAUDE.md PLOP rule.Tests
\", binding key with(, action-ref with\", field-def name with], positive control).idbrejects future version + missing version; referencessupported-versionconstant.em/action-ref-canonical-ns+action-ref-alias(lowercase passthrough, raw user-typed group normalisation, alias strips app. prefix).toolbar-state:autosave-failed?slice.Total: 856 tests / 2476 assertions (+12 assertions vs. main).
Test plan
clj-kondo --lint src test scripts— 0 errors, 0 warningscljfmt check— cleannpx shadow-cljs compile test— 0 failures, 0 errorsnpx shadow-cljs release app— 0 warnings./scripts/check.sh— all four gates green🤖 Generated with Claude Code